Skip to content

Shared: Overhaul the AlertFiltering QLDoc #20047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cklin
Copy link
Contributor

@cklin cklin commented Jul 14, 2025

This PR strengthens the contract for the restrictAlertsTo and the restrictAlertsToExactLocation extensible predicates.

  • restrictAlertsTo is now documented to match any alert location that intersects with a specified line range. (Previously an alert location matches only when its first line is in a specified line range.)
  • restrictAlertsToExactLocation is now documented to match any alert location that wholly contains a specific character range. (Previously an alert location matchis only when it is exactly the same as a specified character range.)

It also contains misc wording changes for clarity.

This commit strengthens the contract for the restrictAlertsTo and the
restrictAlertsToExactLocation extensible predicates.

- restrictAlertsTo is now documented to match any alert location that
  intersects with a specified line range. (Previously an alert location
  matches only when its first line is in a specified line range.)

- restrictAlertsToExactLocation is now documented to match any alert
  location that wholly contains a specific character range. (Previously
  an alert location matchis only when it is exactly the same as a
  specified character range.)

It also contains misc wording changes for clarity.
@cklin
Copy link
Contributor Author

cklin commented Aug 11, 2025

@jbj @hvitved

This is a documentation update follow-up to #19943, concerning support for approximate locations. (That PR changes restrictAlertsTo diff-range location matching from "match the first line of location only" to "test whether the location intersects with the diffrange".)

#19943 treats support for approximate locations as an implementation detail that is not documented (nor expected to be documented.) This PR treats the same support for approximate locations as the new specification for restrictAlertsTo (and restrictAlertsToExactLocation), so that all other users of diff-informed queries, on both the QL authoring side and the CI integration side, can take advantage of this more general (and, I believe, more useful) behavior.

Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! It's great to see the documentation shrinking and becoming clearer while the use cases for diff-informed queries expand just by changing the spec to match the implementation.

Cc @asgerf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants